Fix perf regression in `Read::read_to_end` on short reads due to not checking if the cursor has initialized bytes by asder8215 · Pull Request #158083 · rust-lang/rust

asder8215 · 2026-06-18T14:52:23Z

This PR fixes #158008.

In particular, in #150129, it refactored some code within library/std/io/mod.rs to utilize BorrowedBuf::is_init instead of manually checking read_buf.init_len() == buf_len to see if the read buffer had initialized bytes. However, the BorrowedBuf is never marked or set as init within this function, and I think this portion of the code:

 // SAFETY: These bytes were initialized but not filled in the previous loop
unsafe {
     read_buf.set_init(initialized);
}

was removed by mistake. This PR reverts the changes made by #150129, so that we can mark the BorrowedBuf/read_buf as initialized using BorrowedBuf::set_init if in a previous iteration the cursor has initialized bytes. This would allow max_read_size to not be marked as usize::max if the read buffer contains initialized bytes.

rustbot · 2026-06-18T14:52:29Z

r? @Darksonn

rustbot has assigned @Darksonn.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: @ChrisDenton, libs
@ChrisDenton, libs expanded to 12 candidates
Random selection from Darksonn, Mark-Simulacrum, clarfonthey, jhpratt

Darksonn · 2026-06-18T15:02:01Z

        // Note that we don't track already initialized bytes here, but this is fine
        // because we explicitly limit the read size


This comment was added in #150129. Should it be removed?

View changes since the review

Unsure. Are these comments still relevant @a1phyr?

Well, probably not if you start tracking initialized bytes :)

Darksonn · 2026-06-18T15:12:58Z

@rustbot author

rustbot · 2026-06-18T15:13:02Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

a1phyr

I didn't track uninitialized bytes in my previous MR because I thought it would be to complicated to do properly.

For example, if this MR improve some existing cases, it will not really solve the pathological case you sent in your issue for larger sizes (eg around a million): when you initialized N bytes, on the next round you will have N-1 spare initialized bytes left, so you won't be able to use set_init() (or you could initialize the rest manually).

All in all, it was a trade-off between code complexity, properly handling common cases but having suboptimal (but still acceptable) behavior in weird cases.

View changes since this review

a1phyr · 2026-06-19T23:56:03Z

            }
        };

+        initialized_len = cursor.capacity();


This is true only if read_buf.is_init() (use the boolean below to avoid lifetime issues)

a1phyr · 2026-06-20T00:14:05Z

        let mut read_buf: BorrowedBuf<'_, u8> = spare.into();

+        let buf_unfilled_len = read_buf.capacity() - read_buf.len();
+        if initialized_len == buf_unfilled_len {


This condition is wrong: you compare the old buffer capacity and the new buffer spare capacity, but the start of the buffer has changed since then. It would be less error prone to track initialized bytes counting from the beginning of the Vec.

Yeah, please store the full capacity of the vector, including anything already written.

Darksonn · 2026-06-20T07:13:32Z

@rustbot author

…ecking if the cursor has initialized bytes

asder8215 · 2026-07-05T06:39:59Z

@rustbot ready

Darksonn · 2026-07-06T08:09:33Z

This is still using the wrong condition. Consider this scenario:

We set initialized_len = 10 - 5 where capacity=10, length=5.
The vector is resized and the capacity is now 15.
We read another 5 bytes with is_init=false.
The next iteration compares initialized_len with 15-10 which is true, and considers the buffer initialized.

But in this scenario the buffer was resized and not initialized, so this is wrong. It's really important that initialized_len is such that after the vector is resized, the comparison can never return true until you get a read with is_init=true.

Instead of storing capacity() - len() please just store capacity(). That way, if the capacity changes, then the capacity will no longer be the same.

@rustbot author

asder8215 · 2026-07-06T21:29:03Z

Instead of storing capacity() - len() please just store capacity(). That way, if the capacity changes, then the capacity will no longer be the same.

Just to clarify, resizing only occurs potentially in spots where we call small_probe_reads or in the conditional where buf.len() == buf.capacity() and it does a try_reserve? It should still be safe to use buf_unfilled_len = capacity() - len() on the portion of code where we're converting the spare portion of the buffer into a BorrowedBuf/BorrowedCursor as it shouldn't be able to resize this borrowed buffer/slice with read_buf?

Darksonn · 2026-07-06T21:53:53Z

It's not obvious to me that that's the case. Maybe you are right, but I can't easily follow the logic.

To me, it would be a lot easier to figure out that the code is correct if you stored the capacity. If the vector was reallocated, then I know that the capacity changed, and therefore I know that we will not call set_init().

Darksonn · 2026-07-06T21:58:55Z

I just noticed that you added two lines of code to recompute initialized_len when you call small_probe_reads or try_reserve. In that case it's okay of course, but if you're going to do that, it would be simpler to just store a boolean instead.

The reason I suggested storing the capacity is that you can delete those two lines of code. I'm worried that, in the future, somebody changes this code to add a third place where the vector is reallocated, while forgetting to also update initialized_len in that third place.

asder8215 · 2026-07-06T22:17:36Z

It's not obvious to me that that's the case. Maybe you are right, but I can't easily follow the logic.

I think it does because it seems like the read_buf on BorrowedCursor calls on the default_read_buf, which does the following:

rust/library/std/src/io/mod.rs

Lines 574 to 581 in 36714a9

    
           pub(crate) fn default_read_buf<F>(read: F, mut cursor: BorrowedCursor<'_, u8>) -> Result<()> 
        
           where 
        
               F: FnOnce(&mut [u8]) -> Result<usize>, 
        
           { 
        
               let n = read(cursor.ensure_init())?; 
        
               cursor.advance_checked(n); 
        
               Ok(()) 
        
           }

If I'm understanding it correctly, the advance_checked call here would panic if we try to read more bytes into the unfilled portion of the buffer than what the buffer can handle.

The reason why I asked if it was safe to do buf_unfilled_len = capacity() - len() (in line 476) because that solves the short read issue mentioned in #158008 since from a previous loop it's clear that nothing has been read into the buffer if we have the same unfilled portion length (and that no resizing occurred from earlier small_probe_reads/try_reserve). If I were to do buf_unfilled_len = capacity() at that point there, we're back to the problem that issue had.

Darksonn · 2026-07-07T12:52:32Z

I'm sorry it looks like you're right. I misunderstood the max_read_size logic.

However, I don't think the current logic looks ideal. Let's say that we have a buffer of capacity 1000 and max_read_size is 100, and that every single read is 1 byte long. Then we get this scenario:

Call read(&mut buf[0..100]). After this call, first 100 bytes are initialized.
Call read(&mut buf[1..101]). Before this call, the 101th byte is not initialized, so set_init() is not called. After this call, first 101 bytes are initialized.
Call read(&mut buf[2..102]). Before this call, the 102th byte is not initialized, so set_init() is not called. After this call, first 102 bytes are initialized.
and so on

So it seems like that on short reads, we should repeatedly call read with the same end-index. I.e. we should be calling read(&mut buf[0..100]), then read(&mut buf[1..100]), then read(&mut buf[2..100]) even though this decreases the buffer size each time. That way, the buffer stays initialized between each call.

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 18, 2026

rustbot assigned Darksonn Jun 18, 2026

asder8215 changed the title ~~Fix perf regression in Read::read_to_end on short reads due to not checking if the cursor has initialized bytes~~ Fix perf regression in Read::read_to_end on short reads due to not checking if the cursor has initialized bytes Jun 18, 2026

Darksonn reviewed Jun 18, 2026

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 18, 2026

Darksonn added the A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` label Jun 18, 2026

asder8215 force-pushed the default_read_to_end_mark_init branch from 104baaa to aebb8e1 Compare June 18, 2026 16:03

asder8215 requested a review from Darksonn June 18, 2026 16:05

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 18, 2026

asder8215 force-pushed the default_read_to_end_mark_init branch from aebb8e1 to c6406c5 Compare June 18, 2026 16:06

a1phyr reviewed Jun 20, 2026

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 20, 2026

Fix perf regression in Read::read_to_end on short reads due to not ch…

c0189d9

…ecking if the cursor has initialized bytes

asder8215 force-pushed the default_read_to_end_mark_init branch from c6406c5 to c0189d9 Compare July 5, 2026 06:38

asder8215 requested a review from a1phyr July 5, 2026 06:39

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 5, 2026

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 6, 2026

		// Note that we don't track already initialized bytes here, but this is fine
		// because we explicitly limit the read size

Uh oh!

Uh oh!

Conversation

asder8215 commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jun 18, 2026

Uh oh!

Darksonn Jun 18, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asder8215 Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

a1phyr Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Darksonn commented Jun 18, 2026

Uh oh!

rustbot commented Jun 18, 2026

Uh oh!

a1phyr left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

a1phyr Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

a1phyr Jun 20, 2026

Choose a reason for hiding this comment

Uh oh!

Darksonn Jun 20, 2026

Choose a reason for hiding this comment

Uh oh!

Darksonn commented Jun 20, 2026

Uh oh!

asder8215 commented Jul 5, 2026

Uh oh!

Darksonn commented Jul 6, 2026

Uh oh!

asder8215 commented Jul 6, 2026

Uh oh!

Darksonn commented Jul 6, 2026

Uh oh!

Darksonn commented Jul 6, 2026

Uh oh!

asder8215 commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Darksonn commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

asder8215 commented Jun 18, 2026 •

edited

Loading

Darksonn Jun 18, 2026 •

edited by rustbot

Loading

a1phyr left a comment •

edited

Loading

asder8215 commented Jul 6, 2026 •

edited

Loading